Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/eth_getBlockByNumber and eth_getUncle... nullable fields #341

Merged
merged 22 commits into from
Mar 29, 2021

Conversation

meowsbits
Copy link
Contributor

Rel #334

This is working toward implementing and testing eth_getBlockByNumber for pending blocks achieving complete parity with ethereum/go-ethereum (... and with reference to the docs, which seem to be occasionally a source of noise rather than truth).

One question remaining concerns the totalDifficulty field. Currently (as tested) both core-geth and go-ethereum return null for the pending block. This field is not present in the docs (cited at above ref'd issue). Logic at the API level determines the existence of this field conditionally on an inclTxes boolean. I'm remembering wrangling with this a while ago, but need to do some more digging to get the details again.

Date: 2021-03-15 09:53:13-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-15 09:56:24-05:00
Signed-off-by: meows <[email protected]>
…ro-value, and filled)

Date: 2021-03-15 10:07:04-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-15 10:15:24-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-15 10:16:52-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-15 10:21:40-05:00
Signed-off-by: meows <[email protected]>
…iculty by calculation if DNE

Date: 2021-03-15 10:44:45-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-15 10:55:14-05:00
Signed-off-by: meows <[email protected]>
…incrementing too big or nil

Date: 2021-03-15 11:15:25-05:00
Signed-off-by: meows <[email protected]>
…m/go-ethereum

Date: 2021-03-15 13:35:41-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-15 14:16:06-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-15 14:17:42-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-15 14:48:05-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-15 14:50:05-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-15 14:50:19-05:00
Signed-off-by: meows <[email protected]>
…go-ethereum 1:1 cases

Date: 2021-03-15 15:16:47-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-15 15:25:59-05:00
Signed-off-by: meows <[email protected]>
@meowsbits
Copy link
Contributor Author

meowsbits commented Mar 16, 2021

Ref https://github.com/etclabscore/ethereum.go-ethereum/pull/11/files. This branch shows that ethclient.GetHeader(-1), which should return the pending block, in fact does not. This is a bug in the ethclient implementation (although with roots potentially at types.Header JSON Unmarshal). Note that this branch and the demonstrated bug is at ethereum/go-ethereum.

Though related, this issue is first concerned with the RPC service (ie server) providing valid information (rather than the Go ethclient implementation of a client consuming this data properly).

This is just a little over-the-top and too
ad-hoc to be worth existence.

Date: 2021-03-16 10:33:15-05:00
Signed-off-by: meows <[email protected]>
Date: 2021-03-16 10:55:16-05:00
Signed-off-by: meows <[email protected]>
@meowsbits meowsbits marked this pull request as ready for review March 16, 2021 15:56
hash = &c
}
header.TotalDifficulty = (*hexutil.Big)(s.b.GetTd(ctx, *hash))
header.TotalDifficulty = (*hexutil.Big)(s.b.GetTd(ctx, *header.Hash))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This now matches the ethereum/go-ethereum implementation.

https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1187-L1199

@meowsbits
Copy link
Contributor Author

Re: totalDifficulty.

I've set it to behave equivalently with ethereum/go-ethereum. This means that it will be included (potentially null... under certain conditions...). This might spin off into it's own issue.

@meowsbits meowsbits requested review from iquidus and ziogaschr March 16, 2021 16:05
…ocks

Create a dedicated type for this case, switching
on it conditionally when inclTx is false;
this parameter is false only for uncle blocks.

Date: 2021-03-17 13:08:01-05:00
Signed-off-by: meows <[email protected]>
…ponses

Date: 2021-03-17 13:30:37-05:00
Signed-off-by: meows <[email protected]>
@meowsbits meowsbits changed the title Fix/eth_getBlockByNumber nullable fields Fix/eth_getBlockByNumber and eth_getUncle... nullable fields Mar 17, 2021
@meowsbits
Copy link
Contributor Author

This commit 67ca5fc fixes this issue: #344

@meowsbits
Copy link
Contributor Author

There was a panic at the CI. https://travis-ci.org/github/etclabscore/core-geth/jobs/763301644

At first glance it seems unrelated to this change set, but has now happened twice in a row (after 1 re-run). Will look into it tomorrow.

Copy link
Member

@ziogaschr ziogaschr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@meowsbits meowsbits merged commit f0dd598 into master Mar 29, 2021
@meowsbits meowsbits deleted the fix/eth_getBlockByNumber-nullable-fields branch March 29, 2021 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants